-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Custom authorization failure handler #283
Conversation
Added an option to the http.WithHeaderAuthorization middleware to modify the default response in case of authentication failure.
c5ecf74
to
ea2b482
Compare
kid := "kid-" + t.Name() | ||
// Mock the Clerk API server. We expect requests to GET /jwks. | ||
clerkAPI := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
if r.URL.Path == "/jwks" && r.Method == http.MethodGet { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Shouldn't the path here be "/v1/jwks"
? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API versioning is now determined by a header or query string parameter.
As a result, adding the version in the path has no effect and might change in the future.
We decided to think of "v1" as part of the API's base URL, since in the future API endpoint paths are likely to omit it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved offline, the test Client is not configured with the v1/
path prefix, so this is the request path it accesses.
kid := "kid-" + t.Name() | ||
// Mock the Clerk API server. We expect requests to GET /jwks. | ||
clerkAPI := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
if r.URL.Path == "/jwks" && r.Method == http.MethodGet { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved offline, the test Client is not configured with the v1/
path prefix, so this is the request path it accesses.
Added an option to the http.WithHeaderAuthorization middleware to modify the default response in case of authentication failure.
Fixes #277.